-
Notifications
You must be signed in to change notification settings - Fork 297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: give the application control to validate the app version #1098
Conversation
do you mind elaborating on when coordination would break down? my understanding of this was since we're bumping the version in endblock we'd never call ProcessProposal on a proposal of an unexpected height
afaiu we're doing this in v0.34.x since we're passing the entire header, do you know why did we stop doing this in upstream? |
In the first phase where we use a config file, if two users set different values for when to upgrade then the network would halt. user A would keep proposing a v2 block and user B would reject it and user B would propose a v1 block and user A would reject it. If we introduce this phase then user A proposing a v2 block would still get rejected but user B proposing a v1 block would still be accepted thus the network would avoid halting.
I think the idea was to only give the application the information that is relevant. For example, there's no need for the application to know the evidence hash. |
shouldn't the network halt if nodes do not agree on the same version? That was at least my understanding of what was desired. Is this just a stop gap for signalling? so to clarify, this is proposing that we shouldn't increment the app verison during end block at the configured height, we're only incrementing it once 2/3's prevotes on a proposal with the next version? or we at least have to make sure that we're never using the app's local state to determin the version (like in |
Closing this for now as this isn't the path we want to take going forward with managing version changes |
Description
CometBFT currently validates the app version before
ProcessProposal
is sent. This means coordination needs to happen in the prior block. If coordination breaks down, node operators don't know with which app version to validate the proposed block. Far more robust is if the app version is passed inProcessProposal
. This means that if coordination breaks down to upgrade to v2 we can still continue to approve and commit v1 blocks.Note: It would be even better if instead of
EndBlock
, the app version of the block could be set inPrepareProposal
but I'm not sure if that large of a change is necessary.I'm still thinking about whether
AppVersion
should be removed entirely from tendermint state.PR checklist
.changelog
(we useunclog to manage our changelog)
docs/
orspec/
) and code comments